feat(bytesRead-to-bytesWritten): implement Node.js DEP0108 migration #224
Conversation
There was a problem hiding this comment.
WOW good for first issue 🎉, there are some thing to adjust but on logic/approach it's good for me.
The readme is cool IMO.
There are a missing part which is the dynamic import (also missing on the issue spec my bad 😅).
@brunocroh for that do the resolve binding already work with dynamic import ?
// In CommonJS
async function main() {
const zlib = await import("node:zlib");
const gzip = zlib.createGzip();
const processed = gzip.bytesWritten;
}
main();
// or in ESM
const zlib = await import("node:zlib");
const gzip = zlib.createGzip();
const processed = gzip.bytesWritten;for the red ci you just have to resolve the new npm workspace on the monorepo for that just run npm I 👍
AugustinMauroy
added
the
awaiting author
There was a problem hiding this comment.
Pull Request Overview
This PR implements a Node.js codemod to migrate from the deprecated zlib.bytesRead property to zlib.bytesWritten as part of DEP0108 deprecation handling. The codemod automatically transforms zlib stream usage across various import patterns and variable declarations.
- Implements comprehensive AST transformation logic to track zlib stream variables and replace deprecated property access
- Adds extensive test coverage for different import patterns (CommonJS, ESM, destructured, dynamic) and variable declarations
- Includes documentation and package configuration for the new codemod
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/workflow.ts | Core transformation logic with import tracking, variable resolution, and property replacement |
| workflow.yaml | Workflow configuration defining the AST transformation step |
| tests/input/*.js | Test input files covering various zlib usage patterns |
| tests/expected/*.js | Expected transformation outputs for test validation |
| package.json | Package metadata and dependencies for the codemod |
| codemod.yaml | Codemod registry configuration and metadata |
| README.md | Documentation with examples and usage instructions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
Co-authored-by: Augustin Mauroy <[email protected]>
AugustinMauroy
added
awaiting reviewer
| } | ||
|
|
||
| // 1.b Handle dynamic imports: `await import("node:zlib")` | ||
| const dynamicImports = rootNode.findAll({ rule: { pattern: "const $$$VAR = await import($$$MODULE)" } }); |
There was a problem hiding this comment.
You just need to use $$$(Triple dollar sign) when more than on values will be found, in both case, just one will be existing so it needs to be written with $
const dynamicImports = rootNode.findAll({ rule: { pattern: "const $VAR = await import($MODULE)" } });btw, I think we have an utility that can resolve dynamic imports already, and this usage is not covering all cases, by example, if this line starts with var or let this not will match already.
There was a problem hiding this comment.
I did not find the utility that can resolve dynamic imports but I have covered the rest of the cases for let and var and replaced the $$$ with $ .
I committed the changes.
There was a problem hiding this comment.
I did not resolve convesation. If you know if the utility that can resolve dynamic imports exists and how can I import it, I can use it and modify my code. If not, what i committed work very well and we can keep this version of the code and in this case, I will or if you can resolve conversation.
| Before: | ||
|
|
||
| ```js | ||
| const zlib = require("node:zlib"); | ||
| const gzip = zlib.createGzip(); | ||
| gzip.on("end", () => { | ||
| console.log("Bytes processed:", gzip.bytesRead); | ||
| }); | ||
| ``` | ||
|
|
||
| After: | ||
|
|
||
| ```js | ||
| const zlib = require("node:zlib"); | ||
| const gzip = zlib.createGzip(); | ||
| gzip.on("end", () => { | ||
| console.log("Bytes processed:", gzip.bytesWritten); | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
@alexbit-codemod what was the verdict about diff fenced blocks?
There was a problem hiding this comment.
@JakobJingleheimer FYI I asked for that on codemod slack
Co-authored-by: Jacob Smith <[email protected]>
This pull request implements the codemod to replace deprecated
zlib.bytesReadwith
zlib.bytesWrittenin Node.js transform streams, addressing DEP0108 (#196).Changes included:
.bytesReadare replaced correctly.This PR is part of the feature branch
feat/bytesread-to-bytesWritten.